feat: Support cross-database evaluation with SQLite ground truth#465
feat: Support cross-database evaluation with SQLite ground truth#465arieljassan wants to merge 4 commits into
Conversation
…d truth resolution
|
/gcbrun |
IsmailMehdi
left a comment
IsmailMehdi
left a comment
There was a problem hiding this comment.
Solves a real problem (cross-engine BIRD eval on BQ), but a few correctness issues to address. Inline comments cover the file-specific items. Three cross-cutting points that don't fit a specific line:
No tests for any of the new code. Both sqlite_bridge.py and hybrid_xa_judge.py are trivially unit-testable (in-memory SQLite via sqlite3.connect(":memory:"), mock sys.argv). Without tests the fragile bits flagged inline will silently rot. Minimum I'd want to see: test_get_sqlite_ground_truth_uses_named_db (after the #2 fix), test_compare_result_sets_handles_decimal_vs_float, test_compare_result_sets_ignores_column_names_and_row_order, test_hybrid_xa_judge_main_with_matching_results.
No documentation in docs/configs/run-config.md or dataset-config docs for this new mode. PRs #453 and #459 got the same note — easier to add docs at PR time than to remember later.
PR description "Verification" is functional/manual only — no automated test references. Worth at least pointing to a scenario YAML that exercises the hybrid path so others can repro the test. Also: if you confirm the uv run --isolated import issue (inline comment on hybrid_xa_judge.py:10), worth describing in the PR how the manual verification got past that — I may be misreading the invocation.
|
Maybe this should be built as a tool instead of inline with evals. wdyt ? |
arieljassan
force-pushed
the
feat/bird-xa-benchmark
branch
from
bb25c95 to
1c07b41
Compare
|
Thank you for the review! addressing your general comments: No tests for any of the new code. Added 7 unit tests covering both components under
No documentation in docs/configs/run-config.md or dataset-config docs for this new mode. Added documentation detailing the new configurations:
PR description "Verification" is functional/manual only — no automated test references. Added unit tests and a functional test: .venv_eb/bin/python3 -m unittest evalbench/test/sqlite_bridge_test.py evalbench/test/hybrid_xa_judge_test.pyAnd added a note explaining how the sandbox execution ( |
|
/gcbrun |
| self.client = bigquery.Client(project=self.project_id) | ||
| self.tmp_users = [] | ||
|
|
||
| def ensure_database_exists(self, database_name: str) -> None: |
There was a problem hiding this comment.
dead code, see line 183.
IsmailMehdi
left a comment
IsmailMehdi
left a comment
There was a problem hiding this comment.
LGTM once minor dupe method removed
2 participants
Overview
This PR adds support for running the BIRD benchmark (and similar cross-database SQL evaluations) on BigQuery while using local SQLite databases for ground truth reference execution.
When evaluating AI-generated queries on BigQuery against reference queries written in SQLite syntax (e.g., using
STRFTIMEor SQLite math functions), BigQuery cannot execute the reference queries directly. This update introduces a hybrid bridging mechanism that dynamically resolves reference answers from local SQLite files while evaluating generated queries on BigQuery.Key Changes
evalbench/databases/bigquery.py: Implements the requiredensure_database_existsabstract method onBQDBto fulfill the baseDBclass contract.evalbench/scorers/sqlite_bridge.py&llmrater.py: Adds conditional ground truth resolution. Whengolden_erroroccurs and a hybrid judge is configured,LLMRaterautomatically fetches the true reference execution rows from the local SQLite database.hybrid_xa_judge.py: Adds a self-contained Execution Accuracy (XA) evaluator script forPythonScorer. It normalizes across engine data types (Decimal/Int64vsfloat), ignores column header differences, and compares rows order-independently.Verification
california_schoolsandcard_games).FLOAT64score compatibility).